-
Notifications
You must be signed in to change notification settings - Fork 121
Decode meta_data incorrectly sent as an object
#16141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Decode meta_data incorrectly sent as an object
#16141
Conversation
|
|
|
@staskus not urgent, just when you get to it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a workaround to handle incorrectly formatted meta_data fields in WooCommerce REST API responses. Some plugins transform the expected array format into an object with string indices as keys, breaking the standard JSON structure for Products, Variations, and Orders.
- Adds flexible decoding that supports both array and dictionary formats for metadata
- Updates Product, ProductVariation, and Order models to use the new flexible decoder
- Maintains backward compatibility while fixing visibility of custom fields
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| RELEASE-NOTES.txt | Documents the workaround for incorrectly formatted meta_data |
| MetaDataArray+FlexibleDecoding.swift | New extension providing flexible decoding for MetaData arrays |
| ProductMetadataExtractor.swift | Updated to handle both array and dictionary metadata formats |
| Product.swift | Modified to use flexible metadata decoding |
| ProductVariation.swift | Modified to use flexible metadata decoding |
| Order.swift | Modified to use flexible metadata decoding |
| Test files | Added comprehensive test coverage for both metadata formats |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| // Try to decode as object keyed by index strings | ||
| if let metaDataDict = try? container.decode([String: MetaData].self, forKey: key) { | ||
| return Array(metaDataDict.values) |
Copilot
AI
Sep 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using Array(metaDataDict.values) loses the ordering information from the dictionary keys. Since the keys are string indices ('0', '1', '2'), the metadata should be sorted by these keys to preserve the original order. Consider sorting by converting keys to integers: return metaDataDict.sorted { Int($0.key) ?? 0 < Int($1.key) ?? 0 }.map { $0.value }
| return Array(metaDataDict.values) | |
| return metaDataDict.sorted { (lhs, rhs) in | |
| (Int(lhs.key) ?? 0) < (Int(rhs.key) ?? 0) | |
| }.map { $0.value } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it's true, the docs suggest:
When iterated over, values appear in this collection in the same order as they occur in the dictionary’s key-value pairs.
| newDict.updateValue(objectValue, forKey: objectKey) | ||
| return newDict | ||
| private func getKeyValueDictionary(from metadata: [MetaData]) -> AnyDictionary { | ||
| metadata.reduce(into: AnyDictionary()) { dict, object in |
Copilot
AI
Sep 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The reduce(into:) method is more efficient than the previous reduce() approach since it mutates the accumulator in-place rather than creating new dictionaries on each iteration.
staskus
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Solid work, I appreciate the clear test coverage.
I think it can be clean up slightly by removing the Decodable path from ProductMetadataExtractor.
| } | ||
|
|
||
| // Fallback to empty array | ||
| return [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it would be worth logging a case where metadata decoding fails silently? It could help troubleshoot similar issues more easily.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| } | ||
|
|
||
| // Try to decode as object keyed by index strings | ||
| if let metaDataDict = try? container.decode([String: MetaData].self, forKey: key) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was checking if a plugin could produce [Int: MetaData].self, but it looks unlikely since using numbers as object keys goes against the JSON specs and json_encode() would ensure that the keys are strings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. I'm wary of even doing this much workaround for plugins messing with fields that don't belong to them, to be honest... it seems like a short road to insanity trying to pair that with our type safe models.
It'll continue to fail gracefully even if the format is more broken than this.
|
|
||
| // Try to decode as object keyed by index strings | ||
| if let metaDataDict = try? container.decode([String: MetaData].self, forKey: key) { | ||
| return Array(metaDataDict.values) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it's true, the docs suggest:
When iterated over, values appear in this collection in the same order as they occur in the dictionary’s key-value pairs.
| let container = try decoder.singleValueContainer() | ||
| self.metadata = try container.decode([DecodableDictionary].self) | ||
|
|
||
| // Try to decode as array first (standard format) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was investigating why we need this duplicating logic, and I think the whole init(from decoder: Decoder) throws and Decodable from ProductMetadataExtractor can be removed. I couldn't find any code paths that rely on it. ProductMetadataExtractor is used only through init(metadata:) initializer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, I'll check that.
However, I tried a lot last night to have a sane, non-duplicative approach, and failed hard. It was down to the subtle differences of what data source we were trying to map, and how decoded it already was.
Hopefully the cold light of day will let me delete more of this.
Co-authored-by: Povilas Staskus <[email protected]>
Thanks for the docs. Either way, it doesn't matter – metadata only really makes sense to access by searching for a key anyway. |

Description
Some plugins can break the expected JSON for
meta_dataon Products and Variations. They change the expected array to an object, with string indexes as keys.But it should be an array:
This PR updates our decoding to support this format. Note that we do not re-encode to the incorrect format, so saving custom fields may be broken in this place, but at least they'll be visible.
Steps to reproduce
Ensure you have a product with non-private custom fields (no leading underscore in the key.)
Test information
I've checked viewing Custom Fields on Orders and Products, and Interac refunds (which rely on _charge_id.)
Subscription products also use it, but for the moment I've only used unit tests to verify those.
RELEASE-NOTES.txtif necessary.